Enable JUnit Platform for the KMP jvmTest task#146
Conversation
The `kmp-module` convention never configured a test framework for the JVM target, so `jvmTest` silently discovered zero JUnit 5/Kotest tests in every KMP module. Mirror `module-testing.setupTests()` in the convention (without the `junit-jupiter` engine filter, since `kmp-module` adds the Kotest runner engine to `jvmTest` dependencies) and drop the module-local workaround from `otel-backend`. The `buildSrc` change anticipates the same fix committed to the `config` repository; `./config/pull` will keep the copies in sync once it lands there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d62cfe # Conflicts: # docs/dependencies/dependencies.md # version.gradle.kts
Enabling the JUnit Platform revived tests that had silently not run,
exposing drift between them and production code. Production bugs fixed
(the tests were right): `atConfig()` mapped to `Level.INFO`;
`MetadataHandler.Builder.addRepeatedHandler` rejected repeatable keys
instead of requiring them; `MetadataKey.cast()` returned `null` instead
of the documented `ClassCastException`; repeated-value guards threw
`IllegalStateException` where `IllegalArgumentException` is expected;
`SimpleProcessor` exposed mutable repeated-value lists to handlers;
`ScopedLoggingContext.Builder.run { }` resolved to the stdlib extension
and skipped context installation; lazy messages were evaluated outside
the recursion guard of `AbstractLogger.write`; log timestamps lacked
milliseconds and UTC offset.
Test expectations updated where production was right (logger names,
synthetic lambda method names, eager rate-limiter counting, Kotlin
spread operator for `logVarargs`).
The fixes were authored by the dedicated triage session tracked in
`.agents/tasks/kmp-jvmtest-junit-platform.md` and adopted here after
review; `:logging:jvmTest` passes 232/232 and the full
`build dokkaGenerate` is green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #146 +/- ##
=============================================
+ Coverage 0 67.80% +67.80%
- Complexity 0 464 +464
=============================================
Files 0 109 +109
Lines 0 2578 +2578
Branches 0 403 +403
=============================================
+ Hits 0 1748 +1748
- Misses 0 696 +696
- Partials 0 134 +134 🚀 New features to boost your workflow:
|
Add the missing regression tests: convenience level methods must map to their matching levels (pins the `atConfig()` fix), and `MetadataHandler.Builder.addRepeatedHandler` must reject single-valued keys. Complete the KDoc contracts (`@throws` on `addRepeatedHandler`, `null` pass-through on `MetadataKey.cast`), use `safeToString()` when reporting a failed cast, and apply minor doc and message polish. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The `update-copyright.sh` PostToolUse hook replaced the dual "The Flogger Authors; TeamDev" headers with the plain TeamDev template in the six files edited during the review round. Restore the attribution, keeping the 2026 year. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR enables proper JUnit Platform execution for Kotlin Multiplatform JVM tests (jvmTest) via the kmp-module convention, which in turn surfaced previously-silent test failures and includes a set of production/test fixes to restore correctness.
Changes:
- Configure KMP
jvmTestto run on the JUnit Platform (and remove a now-redundant otel-backend local workaround). - Fix multiple logging/metadata correctness issues that were revealed once tests started running (e.g., level mapping, metadata validation/casting, recursion guarding, timestamp formatting, repeated-metadata immutability).
- Update/extend tests and generated dependency docs; bump snapshot version.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| version.gradle.kts | Bumps published snapshot version. |
| logging/src/jvmTest/kotlin/io/spine/logging/test/LoggingCompatibilityTest.kt | Fixes vararg-array usage in Kotlin test. |
| logging/src/jvmTest/kotlin/io/spine/logging/LogContextSpec.kt | Updates JVM log-context assertions (notably null literal handling). |
| logging/src/jvmTest/kotlin/io/spine/logging/JvmLoggerSpec.kt | Adjusts expectations for synthetic lambda method names and rate-limiter counting semantics. |
| logging/src/jvmTest/kotlin/io/spine/logging/backend/AnyExtsJvmSpec.kt | Aligns test expectation with updated safe-to-string error text. |
| logging/src/commonTest/kotlin/io/spine/logging/context/LogLevelMapSpec.kt | Aligns expectations for Kotlin-qualified logger/class names. |
| logging/src/commonTest/kotlin/io/spine/logging/backend/MetadataHandlerSpec.kt | Adds coverage for rejecting repeated handlers on single-valued keys. |
| logging/src/commonTest/kotlin/io/spine/logging/AbstractLoggerSpec.kt | Adds regression coverage for convenience-level mapping and recursion warning matching. |
| logging/src/commonMain/kotlin/io/spine/logging/MetadataKey.kt | Makes cast() throw ClassCastException (vs returning null) and aligns single/repeated guards to IllegalArgumentException. |
| logging/src/commonMain/kotlin/io/spine/logging/LogPerBucketingStrategy.kt | Fixes bucketing strategies to use JVM class identity/name. |
| logging/src/commonMain/kotlin/io/spine/logging/LogContext.kt | Defers lazy message evaluation into AbstractLogger.write recursion guard. |
| logging/src/commonMain/kotlin/io/spine/logging/context/ScopedLoggingContext.kt | Adds Builder.run(Runnable) member to avoid stdlib run bypassing context installation. |
| logging/src/commonMain/kotlin/io/spine/logging/backend/MetadataProcessor.kt | Prevents mutation of repeated metadata values by handlers via an unmodifiable iterator wrapper. |
| logging/src/commonMain/kotlin/io/spine/logging/backend/MetadataHandler.kt | Fixes repeated-handler validation to require repeatable keys (was inverted). |
| logging/src/commonMain/kotlin/io/spine/logging/AbstractLogger.kt | Fixes atConfig() level mapping; enhances timestamp formatting; adds write(..., prepare=...) hook to run user-code within recursion guard. |
| docs/dependencies/pom.xml | Updates docs POM version to the new snapshot. |
| docs/dependencies/dependencies.md | Regenerates dependency/license report with the new snapshot version. |
| buildSrc/src/main/kotlin/kmp-module.gradle.kts | Configures KMP jvmTest to use JUnit Platform + logging (config-managed build logic). |
| backends/otel-backend/build.gradle.kts | Removes the local jvmTest JUnit Platform workaround now covered by convention. |
| .agents/tasks/kmp-jvmtest-junit-platform.md | Agent task record / verification notes (config-managed). |
alexander-yevsyukov
left a comment
There was a problem hiding this comment.
@claude please see my comments.
Per review, revert the `byClass()`/`byClassName()` strategies to `key::class` / `key::class.qualifiedName!!` and align the tests instead: expect `KClass`-based keys compared by equality (the `::class` wrapper and the qualified name are re-created on each evaluation), and give the `byClassName()` test a member key class, since `qualifiedName` is `null` for local classes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the deprecated Kotlin DSL property delegates (`by getting`,
`by existing`, `by registering`) with the forms recommended by the
deprecation warnings (`getByName`, `named`, `register<T>`), and pass
`project(path)` instead of `Project` objects to the `dokka`
configuration. Drop the `@Suppress("unused")` annotations that only
served the removed delegate `val`s.
The remaining deprecation warnings come from third-party plugins
(Gradle Doctor, Detekt, Kover) and can only be addressed by plugin
updates in a dedicated task.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The new name states what the callback is for — completing the log data within the recursion guard — instead of describing when it runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The `ScopedLoggingContext.Builder.run(Runnable)` member introduced on this branch supersedes the jvmMain extension: both spellings wrap and run the runnable within a new context. Keep the extension deprecated with a `ReplaceWith` for one release cycle instead of removing it, since it is published API. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The "Adding custom levels" example illustrated a custom `CONFIG` at 700 with an `atConfig()` extension — both of which now exist as built-ins (`Level.CONFIG` and `AbstractLogger.atConfig()`). Illustrate with a syslog-style `NOTICE` level instead, which the predefined set genuinely lacks. Also fix the "thew" typo in the same sentence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What changed
The
kmp-moduleconvention plugin never configured a test framework for the KMP JVM target, sojvmTestran without the JUnit Platform and silently discovered zero JUnit 5/Kotest tests in every KMP module (logging,logging-testlib,otel-backend,tests/fixtures). The task passed in ~1 s producing no test reports.buildSrc/src/main/kotlin/kmp-module.gradle.kts— thetasksblock now configuresnamed<Test>("jvmTest")withuseJUnitPlatform()andconfigureLogging(), mirroring whatmodule-testing.setupTests()does for thetesttask of ajvm-module. Deliberate divergences (documented in the KDoc):module-testingitself cannot be applied to a KMP module (it bringsjava-library, which conflicts withkotlin("multiplatform")), and noincludeEngines("junit-jupiter")filter is imposed becausekmp-moduleadds the Kotest runner — a separate JUnit Platform engine — tojvmTestdependencies.backends/otel-backend/build.gradle.kts— removes the module-local workaround that is now redundant..agents/tasks/kmp-jvmtest-junit-platform.md— task record with verification evidence and the triage log.The
buildSrcchange anticipates the identical fix in theconfigrepository (committed there as512c1068on the localaddress-logging-audit-findingbranch; it needs its own PR and the usual float —./config/pullwill keep the copies in sync).Latent failures surfaced and fixed
Enabling the platform revived tests that had silently not run, exposing 25 failures from drift between tests and production code. Production bugs fixed (the tests were right):
AbstractLogger.atConfig()delegated toLevel.INFOinstead ofLevel.CONFIG(now pinned by a regression test covering the whole convenience-method family). TheLevelclass-KDoc "Adding custom levels" example, which usedCONFIGas its custom-level illustration, now uses a syslog-styleNOTICEinstead.MetadataHandler.Builder.addRepeatedHandlerhad inverted validation — it rejected repeatable keys instead of requiring them (drift from Cover customMetadataKey; align factory parameter naming #144; now covered positively and negatively).MetadataKey.cast()returnednullinstead of the documentedClassCastException; repeated-value guards threwIllegalStateExceptionwhereIllegalArgumentExceptionis correct.SimpleProcessor"wrapped" repeated-value lists with a no-op cast, letting handlers mutate them; replaced with a genuinely unmodifiable iterator.ScopedLoggingContext.Builderlacked a memberrun(Runnable), so Kotlin's stdlibrunextension executed blocks without installing the context. The jvmMaincall(Runnable)extension it supersedes is now deprecated with aReplaceWith.AbstractLogger.write, letting throwing/reentranttoString()escape error handling; log timestamps lacked milliseconds and UTC offset.Test expectations were updated where production was right (logger names, synthetic lambda method names, eager rate-limiter counting, Kotlin spread operator for
logVarargs).Verification
:logging:jvmTestexecutes 234 tests, all passing (was: zero discovered, then 25 failing when first enabled).:otel-backend:jvmTestpasses 22/22 without the local workaround;:logging-testlib:jvmTestruns 3 previously-skipped tests../gradlew build dokkaGenerategreen; pre-PR gate PASS (reviewers:kotlin-engineer,spine-code-review,review-docs).0because tests never ran onmaster).🤖 Generated with Claude Code